Conversation
|
See #5609 for more |
| * DNS Interface (Default 8600). Used to resolve DNS queries. TCP and UDP. | ||
|
|
||
| * gRPC API (Default 8302). Currently gRPC is only used to expose Envoy xDS API to Envoy proxies. | ||
| * gRPC API (Default 8502). Currently gRPC is only used to expose Envoy xDS API to Envoy proxies. |
There was a problem hiding this comment.
We probably want to say that it is disabled by default, but is recommended to be run on 8502, similar to what is described here:
https://www.consul.io/docs/agent/options.html#grpc_port
It's kind of a tricky port as we don't listen on it by default. Now that I'm looking at this again I wonder if we should also add https here as well: https://www.consul.io/docs/agent/options.html#https_port.
I think the intention of the "ports used" section is to call out network requirements specifically so I think it makes sense to cover all of the ports there (required or optional).
There was a problem hiding this comment.
Sidecar Service Ports is odd, please let me know if you'd like me to reword it to flow more naturally.
There was a problem hiding this comment.
My suggestion from other PR (post mortem):
- gRPC API (Optional, Defaults to 8502 in
-devmode). Currently gRPC is only used to expose the xDS API to Envoy proxies. It is off by default but port 8502 is a convention used by various tools as the default.
There was a problem hiding this comment.
Maybe a similar clarification of the https port could be added but other than that I think this is fine. HTTPs is not auto-enabled in dev mode so it's not totally the same but the last sentence of the description would be appropriate there.
There was a problem hiding this comment.
I'm with @banks on the suggestions there about the off by default stuff, do you want to make that change @s-christoff?
There was a problem hiding this comment.
I feel like we are maybe being too verbose or saying default too much, this could use some minor tweaks on wording. Let me know what you guys think about the update.
|
Closing, since changes were captured in #5693 |
No description provided.